hmmm I don't understand these changes ...

Just making sure that if the lookup fails and returns nil we get a clean failure from the assert_equals instead of a NoMethodError from calling .content_type on nil. I think I ran into that while testing my code.

This whole dance of checking makes me uneasy... I would be comfortable with a "reaction" to an error JRuby raises when you do a seek but it gives us nothing. I prefer not to own this heuristic.

Perhaps a simple:

MiniMime.preload_dbs = true

Then

if MiniMime.preload_dbs
   @file = StringIO.new(File.read(@path))
end

With some readme data. Very uneasy with the hack of ... seek around file and check that the computer did what you asked it... as a general solution. @headius - it should be exceptioning out if you tell it to seek and it does not seek, this is hiding bugs in JRuby and making them "normal".

it should be exceptioning out if you tell it to seek and it does not seek

I completely agree. However, if we want this code to be compatible with current and past JRuby behavior, we'd still need this "hack". At least the way I wrote it, it should continue working even if JRuby behavior is later changed to raise a reasonable exception (i.e. any IOError or SystemCallError) in this situation.

I suppose a comment explaining why this "dance" is needed might be worth including.

We do not do anything special for seek. It will either use JVM functions to do this (SeekableChannel.position) or a native lseek call.

It does appear to error for an IO.pipe, for example, but I'm unsure if it is the right error:

$ jruby -e 'IO.pipe[0].seek(0)'
Errno::EPIPE: Broken pipe - No message available
    seek at org/jruby/RubyIO.java:1817
  <main> at -e:1

Also for stdin, with a similar error:

$ jruby -e '$stdin.seek(0)'
Errno::ESPIPE: Illegal seek - <STDIN>
    seek at org/jruby/RubyIO.java:1817
  <main> at -e:1

I am a bit confused which resources are not seekable but not raising an error. Just classloader resources? Some other file type for the DB? I think we can extend this error raising to those other resources types if someone can clarify which ones are failing to raise an error here.

I have a JRuby patch that will raise EPIPE for all streams that do not appear to be seekable, including classloader resources:

[] ~/work/jruby93 $ rvm ruby-3.1 do ruby -e 'IO.pipe[0].seek(0)'
-e:1:in `seek': Illegal seek (Errno::ESPIPE)
	from -e:1:in `<main>'

[] ~/work/jruby93 $ rvm ruby-3.1 do ruby -e '$stdin.seek(0)'
-e:1:in `seek': Illegal seek @ rb_io_seek - <STDIN> (Errno::ESPIPE)
	from -e:1:in `<main>'

[] ~/work/jruby93 $ jruby -e 'p File.open("uri:classloader:jruby/kernel/kernel.rb").seek(1)'
Errno::EPIPE: Broken pipe - uri:classloader:jruby/kernel/kernel.rb
    seek at org/jruby/RubyIO.java:1817
  <main> at -e:1

This is a pretty aggressive patch, but might be necessary to make this work properly. By the time we do this seek call, all we have in hand is a java.nio.Channel, and we use the presence of the SeekableChannel supertype to know if seeking is possible. We opted for quiet failure at some point in the past (no idea why) if a channel was not seekable but also not selectable.

I have pushed this as jruby/jruby#7415 but we'll need to evaluate how risky it is before merging and releasing.

cc @enebo

@headius

I am a bit confused which resources are not seekable but not raising an error. Just classloader resources?

Classloader resources are what I've encountered this issue with, since that's what the DB files will end up being when a JRuby application using MiniMime is bundled into a .jar.

We opted for quiet failure at some point in the past (no idea why) if a channel was not seekable but also not selectable.

The silent failure was apparently originally implemented in jruby/jruby@a4cc5ee as a workaround for jruby/jruby#3399. I don't personally think it was a good workaround, but if this behavior is changed, we should probably make sure there's no regression there. (What probably should be done, IMO, would be to fix Rack to properly handle non-seekable resources.)

I commented on my PR as well, but to keep this up-to-date...

I agree that classloader resources, accessible only as InputStream objects that don't support seeking, should raise an error when you attempt to seek on them. If we can be sure this won't affect rack, it is an easy change (and we could narrow my PR to just the resource types in question). Until we can be sure of that, however, we are potentially trading one "broken" behavior for another "broken" behavior.

The simplest fix would be to make all unseekable resources in JRuby raise an error, but there are many resource types in JRuby that would never be encountered in CRuby such as IO resources from in-memory resources that can only be read forward. It is an arguable grey area what is appropriate to do for those.